-
Notifications
You must be signed in to change notification settings - Fork 265
PHPLIB-1736: Add $scoreFusion stage
#1811
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| { | ||
| public function testExample(): void | ||
| { | ||
| $pipeline = new Pipeline(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll bet 20 Bonusly points that the Copilot agent can generate the PHP aggregation on the first try.
Thank you for contributing the PHP library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was a positive comment.
In fact, this file is generated, but it is a frame in which we have to write the code with the PHP aggregation builder. This allows us to validate the functioning of the aggregation builder and check the DX.
The tests/Builder/Stage has many examples of this.
The translation logic is very systematic, which means that an AI agent can easily guess what to write based on the expected extended JSON.
This is a suggestion that you complete this piece of code, but we can also take care of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushed a commit 🤞
❌ 1 Tests Failed:
View the full list of 1 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
5ba6a7c to
dc813d1
Compare
Co-authored-by: Jérôme Tamarelle <[email protected]>
7293492 to
bef5a4b
Compare
|
The remaining test failure seems unrelated and present in other PRs as well. Could / should I do something to increase the code coverage? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds support for MongoDB's $scoreFusion aggregation stage, which enables hybrid search by combining multiple pipelines using relative score fusion. This implementation provides a PHP builder interface for the stage along with comprehensive test coverage.
- Implements the
ScoreFusionStageclass with support for input pipelines, score normalization, combination methods, and score details - Adds factory methods for both static and fluent builder patterns
- Includes test cases with expected JSON output validation
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Builder/Stage/ScoreFusionStage.php | Defines the core ScoreFusionStage class with input, combination, and scoreDetails parameters |
| src/Builder/Stage/FactoryTrait.php | Adds static factory method scoreFusion() to create stage instances |
| src/Builder/Stage/FluentFactoryTrait.php | Adds fluent factory method scoreFusion() for pipeline chaining |
| tests/Builder/Stage/ScoreFusionStageTest.php | Provides test case validating the stage builder against expected pipeline output |
| tests/Builder/Stage/Pipelines.php | Adds expected JSON output enum case for the scoreFusion example test |
| generator/config/stage/scoreFusion.yaml | Configuration file defining the stage specification for code generation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
GromNaN
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could / should I do something to increase the code coverage?
We don't have tests on FluentFactoryTrait. As it's generated code, you can leave it as this.
I restart the failing tests in EVG.
|
Could you rebase, the failing test has been fixed by #1819 |
|
@GromNaN I'm no longer with MongoDB. Perhaps @nirinchev can find someone to take over this PR, alternatively feel free to rebase, merge or close as you see fit 👍 |
|
@kraenhansen thank you for your service 🫡 |
|
Superseded by #1822. |
Merging this PR will:
$scoreFusionas per its documentation.Context: Working on COMPASS-8939 adding auto-complete support for
$scoreFusionto mongosh.